Skip to content

Conversation

vayunbiyani
Copy link
Contributor

After this PR was merged, I switched to master and attempted building ./x.py build --stage 1 library with the config mentioned in the enzyme rustbook but it resulted in some errors tho the config.example.toml build succeeded
The errors were re:

1. Use of ref in match patterns

The errors were related to match ergonomics in Rust 2024, where ref is no longer needed when matching on references. Examples:

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:136:31
    |
136 |             Annotatable::Item(ref iitem) => {
    |                               ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:136:13
    |
136 |             Annotatable::Item(ref iitem) => {
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
136 -             Annotatable::Item(ref iitem) => {
136 +             Annotatable::Item(iitem) => {
    |

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:146:36
    |
146 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |                                    ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:146:13
    |
146 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
146 -             Annotatable::AssocItem(ref assoc_item, _) => {
146 +             Annotatable::AssocItem(assoc_item, _) => {
    |

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:174:31
    |
174 | ...       Annotatable::Item(ref iitem) => (iitem.vis.clone(), iitem.ide...
    |                             ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:174:13
    |
174 | ...   Annotatable::Item(ref iitem) => (iitem.vis.clone(), iitem.ident.c...
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
174 -             Annotatable::Item(ref iitem) => (iitem.vis.clone(), iitem.ident.clone()),
174 +             Annotatable::Item(iitem) => (iitem.vis.clone(), iitem.ident.clone()),
    |

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:175:36
    |
175 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |                                    ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:175:13
    |
175 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
175 -             Annotatable::AssocItem(ref assoc_item, _) => {
175 +             Annotatable::AssocItem(assoc_item, _) => {
    |

error: could not compile `rustc_builtin_macros` (lib) due to 4 previous errors
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:19:39

2. the use of external C blocks without unsafe in compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs (I don't have the error message handy)

The first commit fixes the errors above


Additional Improvement:

@ZuseZ4 suggested we consolidate the variants under #[cfg(llvm_enzyme)] and #[cfg(not(llvm_enzyme))] by conditionally checking for cfg!(llvm_enzyme) instead. This way, the autodiff code is compiled but not executed avoiding such regressions

r? @ZuseZ4
cc: @oli-obk

@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2025

Failed to set assignee to ZuseZ4: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 27, 2025
@oli-obk oli-obk self-assigned this Feb 27, 2025
@rust-log-analyzer

This comment has been minimized.

@vayunbiyani vayunbiyani force-pushed the fix-enzyme-build-errors branch from e2d7d42 to c85f038 Compare February 27, 2025 14:03
@oli-obk oli-obk marked this pull request as ready for review February 27, 2025 14:14
@oli-obk
Copy link
Contributor

oli-obk commented Feb 27, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 27, 2025

📌 Commit c85f038 has been approved by oli-obk

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Feb 27, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#136424 (fix: overflowing bin hex)
 - rust-lang#136824 (solver cycles are coinductive once they have one coinductive step)
 - rust-lang#137220 (Support `rust.channel = "auto-detect"`)
 - rust-lang#137712 (Clean up TypeckResults::extract_binding_mode)
 - rust-lang#137713 (Fix enzyme build errors)
 - rust-lang#137748 (Fix method name in `TyCtxt::hir_crate()` documentation)
 - rust-lang#137778 (update enzyme to handle range metadata)
 - rust-lang#137780 (Fix typo in query expansion documentation)
 - rust-lang#137788 (Bump `rustc_{codegen_ssa,llvm}` `cc` to 1.2.16 to fix `x86` Windows jobs on newest Windows SDK)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d65f568 into rust-lang:master Feb 28, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 28, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
Rollup merge of rust-lang#137713 - vayunbiyani:fix-enzyme-build-errors, r=oli-obk

Fix enzyme build errors

After [this PR](rust-lang#136428) was merged, I switched to master and attempted building `./x.py build --stage 1 library` with the config mentioned in the enzyme rustbook but it resulted in some errors tho the config.example.toml build succeeded
The errors were re:
### 1. Use of ref in match patterns
The errors were related to match ergonomics in Rust 2024, where ref is no longer needed when matching on references. Examples:
```
error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:136:31
    |
136 |             Annotatable::Item(ref iitem) => {
    |                               ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:136:13
    |
136 |             Annotatable::Item(ref iitem) => {
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
136 -             Annotatable::Item(ref iitem) => {
136 +             Annotatable::Item(iitem) => {
    |

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:146:36
    |
146 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |                                    ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:146:13
    |
146 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
146 -             Annotatable::AssocItem(ref assoc_item, _) => {
146 +             Annotatable::AssocItem(assoc_item, _) => {
    |

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:174:31
    |
174 | ...       Annotatable::Item(ref iitem) => (iitem.vis.clone(), iitem.ide...
    |                             ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:174:13
    |
174 | ...   Annotatable::Item(ref iitem) => (iitem.vis.clone(), iitem.ident.c...
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
174 -             Annotatable::Item(ref iitem) => (iitem.vis.clone(), iitem.ident.clone()),
174 +             Annotatable::Item(iitem) => (iitem.vis.clone(), iitem.ident.clone()),
    |

error: binding modifiers may only be written when the default binding mode is `move`
   --> compiler/rustc_builtin_macros/src/autodiff.rs:175:36
    |
175 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |                                    ^^^ binding modifier not allowed under `ref` default binding mode
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
note: matching on a reference type with a non-reference pattern changes the default binding mode
   --> compiler/rustc_builtin_macros/src/autodiff.rs:175:13
    |
175 |             Annotatable::AssocItem(ref assoc_item, _) => {
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this matches on type `&_`
help: remove the unnecessary binding modifier
    |
175 -             Annotatable::AssocItem(ref assoc_item, _) => {
175 +             Annotatable::AssocItem(assoc_item, _) => {
    |

error: could not compile `rustc_builtin_macros` (lib) due to 4 previous errors
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:19:39
```
### 2. the use of external C blocks without unsafe in compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs (I don't have the error message handy)

The first commit fixes the errors above

---
## Additional Improvement:
`@ZuseZ4` suggested we consolidate the variants under `#[cfg(llvm_enzyme)]` and `#[cfg(not(llvm_enzyme))]` by conditionally checking for `cfg!(llvm_enzyme)` instead. This way,  the autodiff code is compiled but not executed avoiding such regressions

r? `@ZuseZ4`
cc: `@oli-obk`
@ZuseZ4 ZuseZ4 added the F-autodiff `#![feature(autodiff)]` label Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-autodiff `#![feature(autodiff)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants